-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CheckSchema: Verify intermediate schema upgrades #789
base: main
Are you sure you want to change the base?
Conversation
When skipping a version for an Icinga DB upgrade, all intermediate upgrade steps must be taken. While this is already stated in the documentation, it might be overlooked. This happened for one community user, upgrading from v1.1.0 to v1.2.0, skipping the intermediate schema upgrade for v1.1.1. > https://community.icinga.com/t/icingadb-failing-exactly-5-minutes-after-start/13955 First, the necessity for all upgrades in their release order was made more prominent in the documentation, hoping that less users would ignore this when skimming the upgrade docs. However, the real change here is adding another check to the CheckSchema function, verifying that all schema upgrades between the lowest known version and the highest known version in the icingadb_schema table exists. If an intermediate schema upgrade was skipped, as in the thread above, this raises a descriptive error.
2c23f63
to
4b6ec3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Al2Klimov had an alternative idea: Is it possible to add a check/throw statement at the beginning of each schema upgrade file? That way, also manual migrations would be guarded.
While I really like this idea, I just had difficulties implementing this in a MySQL schema upgrade file in a compact, not-over-engineered way. First, MySQL's The shortest MySQL version I came up with looks like the following. DROP PROCEDURE IF EXISTS schema_upgrade;
DELIMITER //
CREATE PROCEDURE schema_upgrade(expected_version SMALLINT, new_version SMALLINT)
BEGIN
SELECT version INTO @latest_version FROM icingadb_schema ORDER BY id DESC LIMIT 1;
IF @latest_version != expected_version THEN
SIGNAL SQLSTATE '45000'
SET MESSAGE_TEXT = 'Unexpected latest schema version. Are all intermediate upgrades applied?';
END IF;
INSERT INTO icingadb_schema (version, timestamp) VALUES (new_version, UNIX_TIMESTAMP() * 1000);
END //
DELIMITER ;
CALL schema_upgrade(6, 7);
DROP PROCEDURE schema_upgrade;
-- Actual schema upgrades is here.
SELECT 23; While this still feels a bit bloated - and PostgreSQL is yet to come -, this needs to be copied to every new schema upgrade, which is a manual task we must not forget. Otherwise, the Don't get me wrong, I like the idea of having this logic in the database, but unless someone points me into a more practical direction, I am uncertain. |
Including the procedure directly in the baseline schema for use in future upgrades makes sense to me. But I haven't thought about when you can actually rely on it in a schema upgrade, as it needs to be introduced with a schema upgrade first. I have some ideas that we can discuss tomorrow. |
FWIW, division by zero didn't work as expected:
The idea was, if it's missing, count(*) is zero, so 1/0. But I only got a warning, even with ERROR_FOR_DIVISION_BY_ZERO. |
@Al2Klimov, yes, I also had to realize that the MySQL CLI continues execution regardless of errors. That's another reason why I came up with the procedure, explicitly signaling the error. This thread also contains some insights: https://stackoverflow.com/questions/773889/way-to-abort-execution-of-mysql-scripts-raising-error-perhaps |
When skipping a version for an Icinga DB upgrade, all intermediate upgrade steps must be taken. While this is already stated in the documentation, it might be overlooked.
This happened for one community user, upgrading from v1.1.0 to v1.2.0, skipping the intermediate schema upgrade for v1.1.1.
First, the necessity for all upgrades in their release order was made more prominent in the documentation, hoping that less users would ignore this when skimming the upgrade docs.
However, the real change here is adding another check to the CheckSchema function, verifying that all schema upgrades between the lowest known version and the highest known version in the icingadb_schema table exists. If an intermediate schema upgrade was skipped, as in the thread above, this raises a descriptive error.